Skip to content

Support anchored and smoothed modifiers#696

Open
sylr wants to merge 9 commits intothanos-io:mainfrom
sylr:feat/anchored-smoothed-modifiers
Open

Support anchored and smoothed modifiers#696
sylr wants to merge 9 commits intothanos-io:mainfrom
sylr:feat/anchored-smoothed-modifiers

Conversation

@sylr
Copy link
Copy Markdown

@sylr sylr commented Mar 20, 2026

Summary

Implements Prometheus proposal 0052 — the anchored and smoothed modifiers for range vector selectors.

Closes #630

  • anchored: Pins boundary values to real observed samples, giving exact integer results for increase() with no extrapolation artifacts
  • smoothed: Interpolates boundary values between nearest samples, giving robust rate estimation that handles scrape jitter
  • Function whitelists enforced: anchored supports rate, increase, delta, resets, changes; smoothed supports rate, increase, delta
  • Gated behind EnableExtendedRangeSelectors engine option (sets parser.EnableExtendedRangeSelectors)
  • Counter-reset handling during smoothed interpolation matches upstream Prometheus v0.310.0 semantics
  • hints.Range preserved as original selector range (not widened), matching Prometheus behavior for backend pushdown

Changes across engine layers

Layer File Change
Engine engine/engine.go EnableExtendedRangeSelectors option, parser flag
Logical plan logicalplan/plan.go Extended storage time ranges
Execution execution/execution.go Function whitelist enforcement, query window extension
Execution execution/parse/functions.go Whitelist maps
Ring buffer ringbuffer/functions.go extendedRangeRate with boundary pick/interpolate and counter-reset correction
Ring buffer ringbuffer/generic.go NewAnchored/NewSmoothed constructors
Storage storage/prometheus/matrix_selector.go Thread anchored/smoothed through operator
Storage storage/prometheus/scanners.go Pass modifier flags

Test plan

  • 18 test cases comparing thanos engine output against upstream Prometheus engine (rate, increase, delta, resets, changes with anchored/smoothed, counter resets, multiple series, quadratic data)
  • Function whitelist enforcement tests (unsupported functions rejected, supported functions accepted)
  • SelectHints regression test asserting hints.Range equals original selector range for standard, anchored, and smoothed queries
  • All existing tests pass (pre-existing fuzz test flake excluded)

🤖 Generated with Claude Code

@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from 5a81567 to 5082016 Compare March 20, 2026 08:18
@fpetkovski fpetkovski marked this pull request as ready for review March 20, 2026 08:41
@fpetkovski fpetkovski marked this pull request as draft March 20, 2026 08:41
@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from 5082016 to f497bd3 Compare March 20, 2026 09:29
@sylr sylr marked this pull request as ready for review March 20, 2026 09:30
sylr and others added 4 commits March 20, 2026 10:31
Implement Prometheus proposal 0052 (extended range selectors) for the
thanos promql-engine. The anchored modifier pins boundary values to real
samples, while the smoothed modifier interpolates boundary values,
eliminating extrapolation artifacts in rate/increase/delta calculations.

Closes thanos-io#630

Changes across the engine layers:

- engine: add EnableExtendedRangeSelectors option, set parser flag
- logicalplan: extend storage time ranges for anchored/smoothed selectors
- execution: enforce function whitelists (anchored: rate, increase, delta,
  resets, changes; smoothed: rate, increase, delta), extend hint ranges
- ringbuffer: add Anchored/Smoothed fields to FunctionArgs, implement
  extendedRangeRate with pickOrInterpolateLeft/Right boundary logic and
  counter-reset correction
- storage/prometheus: thread anchored/smoothed through matrixSelector,
  use ext-lookback buffers, extend maxt for smoothed post-range samples

Constraint: parser.EnableExtendedRangeSelectors is a global variable in
prometheus/parser — can only be set to true (additive) to avoid races
Rejected: Streaming ring buffers for anchored/smoothed | they lack
ext-lookback and boundary interpolation support
Confidence: high
Scope-risk: narrow
Not-tested: native histograms with anchored/smoothed (rejected upstream too)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
…thed

Address review feedback on the anchored/smoothed implementation:

1. Counter-reset at boundaries: interpolateAt now handles counter resets
   during smoothed interpolation (left edge zeros y1, right edge adds y1
   to y2), matching upstream Prometheus' interpolate function.
   correctForCounterResets now checks the right boundary value against
   the last interior sample, preventing impossible negative increase
   values when a reset occurs at the range boundary.

2. hints.Range semantics: the widened query window (for ext-lookback) is
   now only applied to hints.Start/End. hints.Range stays equal to the
   original selector range so backends using it for resolution selection
   or pushdown see the true range, matching Prometheus behavior.

3. Added regression test for smoothed increase/rate with counter reset
   at the range boundary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Assert that hints.Range passed to NewMatrixSelector always equals the
original selector range, not the widened query window. Uses a spy
scanner that captures hints and verifies the contract for standard,
anchored, and smoothed selectors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Remove the leftEdge parameter from interpolateAt. Counter resets during
interpolation now always set y1=0 for both edges, matching the upstream
simplification in prometheus/prometheus v0.310.0 (promql/functions.go).

The old v0.308.0 behavior was asymmetric: left edge zeroed y1, right
edge added y1 to y2. The new unified approach is cleaner and correctly
models counters as starting from 0 post-reset regardless of which
boundary is being interpolated.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from f497bd3 to 5e88f32 Compare March 20, 2026 09:31
sylr and others added 2 commits March 20, 2026 10:51
When step intervals don't align with sample intervals, the sample
needed as the ext lookback boundary may be trapped in lastSample from
the previous step and not pushed back to the buffer (since its timestamp
is <= mint). For anchored/smoothed selectors, push lastSample back into
the buffer before Reset so the ext lookback retention logic can decide
whether to keep it. This is scoped to anchored/smoothed only to avoid
changing existing x-function behavior.

Also adds a fuzz test (FuzzAnchoredSmoothedModifiers) that generates
random data and step parameters, testing all anchored/smoothed function
combinations against the upstream Prometheus engine. The fuzzer found
this bug within seconds.

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix a potential slice panic in correctForCounterResets when all interior
samples are excluded by the boundary narrowing (corrFirst > corrLast
with corrLast = -1). Clamp corrLast to corrFirst-1 to produce a valid
empty slice while still allowing the right-boundary counter-reset check
to fire.

Also remove a duplicate mint computation in selectPoints (dead code),
and document the parser.EnableExtendedRangeSelectors process-global
limitation in engine.go.

Found by architect review.

Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread ringbuffer/functions.go Outdated
Comment thread engine/engine.go Outdated
"github.com/prometheus/prometheus/promql/promqltest"
promstorage "github.com/prometheus/prometheus/storage"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update engine_test.go to exclude the extended range test cases?

"testdata/extended_vectors.test", // experimental anchored/smoothed modifiers unsupported

That should cover the same test cases from upstream and validate this implementation

@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from ed93cda to 857e3f2 Compare March 24, 2026 08:49
sylr and others added 2 commits March 24, 2026 10:42
… fix anchored/smoothed gaps

- Replace "proposal 0052" references with actual GitHub link to
  prometheus/proposals/0052-extended-range-selectors-semantics.md
- Remove extended_vectors.test from acceptance test skip list and
  enable EnableExtendedRangeSelectors in TestPromqlAcceptance
- Fix error message format to match Prometheus ("can only be used
  with: X, Y - not with Z") and defer validation errors to eval time
  via deferredError operator for expect-fail-msg compatibility
- Implement smoothed instant vector interpolation (selectPointSmoothed)
  matching Prometheus smoothSeries behaviour
- Fix anchored changes/resets on sparse data by retaining an additional
  lookback sample in the ring buffer and adding pickFirstSampleIndex
  to start comparisons from the correct boundary sample

Constraint: error messages must match Prometheus exactly for expect-fail-msg tests
Constraint: smoothed instant vectors need forward-looking samples for interpolation
Rejected: extending mint in matrix selector for anchored | would over-widen sample window for all functions
Confidence: high
Scope-risk: moderate
Not-tested: smoothed instant vectors with native histograms (returns no data, not an error)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
… flag

The Prometheus acceptance test framework toggles the process-global
EnableExtendedRangeSelectors flag during its run. Extended range tests
used t.Parallel(), causing them to overlap and see the flag in the
wrong state — producing data races and spurious "expected error, got
nothing" failures.

- Remove t.Parallel() from all extended range tests so they cannot
  overlap with the acceptance test's flag toggling
- Fix whitelist test to check errors at Exec() time (deferred error
  pattern) instead of at NewInstantQuery() time
- Remove redundant flag manipulation in TestPromqlAcceptance cleanup

Constraint: parser.EnableExtendedRangeSelectors is a process-global
Rejected: sync.Once in engine | flag is legitimately toggled by acceptance tests
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@sylr sylr force-pushed the feat/anchored-smoothed-modifiers branch from 857e3f2 to 203abc8 Compare March 24, 2026 12:45
Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
@sylr
Copy link
Copy Markdown
Author

sylr commented Apr 6, 2026

@yeya24 what do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support anchored and smoothed modifier

2 participants